Respect TEST_RANDOM_SEED when --rng-seed is absent; add SelfTest & approvals #3024
Conversation
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## devel #3024 +/- ##
==========================================
- Coverage 90.93% 90.93% -0.00%
==========================================
Files 201 201
Lines 8681 8691 +10
==========================================
+ Hits 7894 7903 +9
- Misses 787 788 +1 🚀 New features to boost your workflow:
|
|
can you take a look?? |
…v; add SelfTest and update approvals
aryanrahar
force-pushed
the
feat/bazel-test-random-seed
branch
from
1a1669b to
1033955
Compare
|
I've looked into the failing check and see that I missed adding the license header to the new test file. I have pushed a fix for that now. |
|
can you approve the changes? |
|
can you take a look? |
|
??? |
horenmar
left a comment
horenmar
left a comment
There was a problem hiding this comment.
The precedence of CLI -> ENV -> default is probably the correct design.
However, the precedence is not tested, and the code is not ready to be merged.
|
|
||
| CATCH_TRY { | ||
| if (!m_configData.rngSeedSpecified) { | ||
| if (const char* v = std::getenv("TEST_RANDOM_SEED")) { |
There was a problem hiding this comment.
std::getenv is not supported on all platforms. See how other Bazel config options do this.
| set_tests_properties(BazelEnvSeedTest PROPERTIES | ||
| ENVIRONMENT "TEST_RANDOM_SEED=12345" | ||
| COST 1 | ||
| ) |
There was a problem hiding this comment.
There is no reason to specify COST manually.
Also, Bazel env vars are only meant to be used if the main Bazel env var is present to enable Bazel support.
| const char* v = std::getenv("TEST_RANDOM_SEED"); | ||
| if (!v) { | ||
|
|
||
| SUCCEED("TEST_RANDOM_SEED not set; skipping env-specific check"); | ||
| return; | ||
| } |
There was a problem hiding this comment.
No. Don't make conditionally disabled tests for something that is easy-enough to check without.
The used rng seed is printed to stdout, so the test can instead read that.
| #include <catch2/reporters/catch_reporter_registrars.hpp> | ||
|
|
||
|
|
||
|
|
| return ParserResult::ok(ParseResultType::Matched); | ||
| } else if (seed == "random-device") { | ||
| config.rngSeed = generateRandomSeed(GenerateFrom::RandomDevice); | ||
| config.rngSeedSpecified = true; |
2 participants
What
Make Catch2 honor the environment variable TEST_RANDOM_SEED only when --rng-seed is not provided on the command line.
Preserve precedence: CLI (--rng-seed) > environment (TEST_RANDOM_SEED) > default generation (time/random device).
Add a SelfTest that verifies both behaviors and update Approval baselines accordingly.
Why
Requested in #3021 to enable Bazel/CI workflows that supply the seed via env var, while still letting developers override it explicitly with --rng-seed.
How
In Session::runInternal, read TEST_RANDOM_SEED and set m_configData.rngSeed iff the CLI did not specify a seed.
Parse the env seed as unsigned decimal; ignore the env var if it is missing or malformed.
Add tests/SelfTest/BazelEnvSeed.tests.cpp:
If TEST_RANDOM_SEED is not set: test logs a SUCCEED message and exits (so Approvals can run without per-machine env noise).
If TEST_RANDOM_SEED is set: asserts that reporter prints the same seed and that cfg->rngSeed() matches it.
With env set and --rng-seed 42: asserts reporter shows 42 (CLI wins).
Register the test in tests/CMakeLists.txt (BazelEnvSeedTest) and update Approval baselines to account for one extra test case in outputs.
Notes
No public API changes, no amalgamated files modified.
Minimal, localized changes; no measurable perf impact.
GitHub Issues
Implements the request in #3021
If acceptable, this PR closes #3021.
Changed files (summary)
src/catch2/catch_session.cpp — apply env seed when CLI seed absent.
(Aux plumbing where applicable to carry seed through unchanged.)
tests/SelfTest/BazelEnvSeed.tests.cpp — new SelfTest (with license header).
tests/CMakeLists.txt — add source and BazelEnvSeedTest + (optional) CLI-override test.
tests/SelfTest/Baselines/* — approvals updated for the new test case.
Testing
ctest basic preset: ✅
BazelEnvSeedTest:
SelfTest "[bazel-seed]" → passes (no env set).
TEST_RANDOM_SEED=777 SelfTest "[bazel-seed]" → prints Randomness seeded to: 777 and matches config.
TEST_RANDOM_SEED=777 SelfTest "[bazel-seed]" --rng-seed 42 → prints 42 (CLI precedence).